generator: fix off-by-one in mavlink_get_msg_entry #343
Conversation
|
Who's doing the fuzz testing? It has been talked about for a while, didn't know anyone was actually doing it yet. Great find! |
|
@magicrub I started doing some fuzz testing here: https://github.com/Auterion/mavlink-fuzz-testing |
When parsing and discovering a big enough (and possibly invalid) msgid, we ended up accessing the last + 1 element of mavlink_message_crcs. This was found by fuzz testing against mavlink_parse_char with address sanitizer enabled.
julianoes
force-pushed
the
fix-msg-entry-off-by-one
branch
from
0806b4b to
34308c1
Compare
There was a problem hiding this comment.
The change looks good to me (high should be the index of the last array position to be checked, not the size).
I have to say though that I don't really comprehend the algorithm used here, it seems to mix two versions:
- the standard binary search where you check and return (in the loop) if the middle is the right one, and
- the alternative where after the loop you check if the chosen index is the right one
|
Do you need more changes from me or can this get in? |
|
Sorry for the bump. Could this go in? |
|
Bump 🥺. |
|
good, find, thanks! |
|
Thanks @tridge! |
Includes at least these two bugfixes: ArduPilot/pymavlink#343 ArduPilot/pymavlink#344
Includes at least these two bugfixes: ArduPilot/pymavlink#343 ArduPilot/pymavlink#344
When parsing and discovering a big enough (and possibly invalid) msgid, we ended up accessing the last + 1 element of mavlink_message_crcs.
This was found by fuzz testing against mavlink_parse_char with address sanitizer enabled.